feat(grid): edit-config add blurOutside #3521
Conversation
WalkthroughA new feature was introduced that allows customizing the exit logic of grid cell editing through a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GridComponent
participant blurOutsideFunction
User->>GridComponent: Clicks outside editable cell (e.g., on scrollbar)
GridComponent->>blurOutsideFunction: Calls blurOutside({ cell, event, $table })
alt blurOutside returns true
GridComponent-->>User: Editing remains active
else blurOutside returns false
GridComponent-->>User: Editing ends (blur occurs)
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/vue/src/grid/src/body/src/body.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/vue/src/grid/src/mobile-first/index.vueOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (6)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
examples/sites/demos/apis/grid.js (2)
3566-3567: Consistent version annotation style for activeStrictly
The comment uses full-width parentheses and only Chinese text; please switch to half-width parentheses, add an English translation, and match the style of other version notes.
3568-3569: DocumentblurOutsidebehavior and usage
Provide an English description alongside the Chinese comment, clarify when this hook is invoked, and consider adding a defaultValue or fallback note to indicate its optional nature.examples/sites/demos/pc/app/grid/edit/scrollbar-not-blur.spec.ts (1)
18-27: Minor resiliency: Fail fast ifboundingBox()returnsnull
boundingBox()can legally returnnullwhen the element is detached or hidden, which would crash the destructuring assignment.
A quick guard keeps the test failure message focused and avoids an unhandled exception.- const { x, y, height } = await bodyWrapper.boundingBox() + const box = await bodyWrapper.boundingBox() + expect(box).not.toBeNull() + const { x, y, height } = /** @type {Required<typeof box>} */ (box)examples/sites/demos/pc/app/grid/edit/scrollbar-not-blur-composition-api.vue (1)
81-109: Micro-optimisation: reuse the body wrapper reference
isScrollBarrepeatedly performstableElm.querySelector('.tiny-grid__body-wrapper').
Caching the wrapper once avoids a DOM lookup on every click.- if (element !== tableElm.querySelector('.tiny-grid__body-wrapper')) { + const bodyWrapper = tableElm.querySelector('.tiny-grid__body-wrapper') + if (element !== bodyWrapper) { return false }examples/sites/demos/pc/app/grid/edit/scrollbar-not-blur.vue (1)
90-117: Duplicate DOM query – same suggestion as the Composition API versionConsider caching the
bodyWrappernode instead of callingquerySelectortwice per click.
Apply the same optimisation shown in the Composition-API variant for consistency.examples/sites/demos/pc/app/grid/webdoc/grid-edit.js (2)
103-114: Add Composition-API demo file (consistency with other demos).The PR introduces both
scrollbar-not-blur.vueandscrollbar-not-blur-composition-api.vue, yet only the Options-API file is referenced here. Most grid demos list both flavours when both exist; omitting the Composition-API variant means it will not appear in the docs site’s navigation, which is easy to miss.- codeFiles: ['edit/scrollbar-not-blur.vue'] + codeFiles: [ + 'edit/scrollbar-not-blur.vue', + 'edit/scrollbar-not-blur-composition-api.vue' + ]Including both keeps the catalogue symmetrical with other dual-implementation demos and prevents accidental doc drift.
106-113: Tighten the English label and description.“Scroll not blur” is a bit ambiguous; “Scrollbars don’t blur” or “Keep focus when scrolling” reads more naturally. Likewise, the EN description could mirror the CN wording for clarity.
- name: { 'zh-CN': '行编辑滚动不失焦', 'en-US': 'Scroll not blur' }, + name: { 'zh-CN': '行编辑滚动不失焦', 'en-US': 'Keep focus when scrolling' }, ... - '<p>Configure <code>blurOutside</code> of <code>edit-config</code> to customize the exit logic of the editing state. </p>\n' + '<p>Use the <code>blurOutside</code> option of <code>edit-config</code> to customise when editing should exit. </p>\n'Purely copy-text, but small wording tweaks improve developer comprehension in the docs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/sites/demos/apis/grid.js(1 hunks)examples/sites/demos/pc/app/grid/edit/scrollbar-not-blur-composition-api.vue(1 hunks)examples/sites/demos/pc/app/grid/edit/scrollbar-not-blur.spec.ts(1 hunks)examples/sites/demos/pc/app/grid/edit/scrollbar-not-blur.vue(1 hunks)examples/sites/demos/pc/app/grid/webdoc/grid-edit.js(1 hunks)packages/vue/src/grid/src/table/src/methods.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (1)
examples/sites/demos/pc/app/grid/webdoc/grid-edit.js (1)
115-118: Reminder: exposeblurOutsidein the API section.The new prop is demonstrated here but the
apisarray below still shows emptyprops. Ensureexamples/sites/demos/apis/grid.js(or corresponding Markdown) listsblurOutsideso it’s searchable in the docs.No code change in this file—just a heads-up.
| if (typeof editConfig.blurOutside === 'function') { | ||
| return Boolean(editConfig.blurOutside({ cell: args.cell, event, $table: this })) | ||
| } | ||
|
|
There was a problem hiding this comment.
Add a null-safe guard before dereferencing editConfig
editConfig is destructured from this but is not guaranteed to exist (the table can run without edit-config).
Directly evaluating typeof editConfig.blurOutside throws a TypeError when editConfig is undefined.
- if (typeof editConfig.blurOutside === 'function') {
- return Boolean(editConfig.blurOutside({ cell: args.cell, event, $table: this }))
+ const fn = typeof editConfig?.blurOutside === 'function' ? editConfig.blurOutside : null
+ if (fn) {
+ return Boolean(fn({ cell: args.cell, event, $table: this }))
}This matches the defensive style already used elsewhere in the codebase (e.g. many places use optional chaining) and prevents a rare-but-nasty runtime crash.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (typeof editConfig.blurOutside === 'function') { | |
| return Boolean(editConfig.blurOutside({ cell: args.cell, event, $table: this })) | |
| } | |
| const fn = typeof editConfig?.blurOutside === 'function' ? editConfig.blurOutside : null | |
| if (fn) { | |
| return Boolean(fn({ cell: args.cell, event, $table: this })) | |
| } |
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/table/src/methods.ts around lines 1136 to 1139, add
a null-safe check for editConfig before accessing its blurOutside property to
prevent a TypeError when editConfig is undefined. Use optional chaining or an
explicit check to ensure editConfig exists before evaluating typeof
editConfig.blurOutside. This will align with the defensive coding style used
elsewhere and avoid runtime crashes.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit